Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update to qe-tools 2.0.0rc1. #529

Merged
merged 1 commit into from
Oct 8, 2020
Merged

Update to qe-tools 2.0.0rc1. #529

merged 1 commit into from
Oct 8, 2020

Conversation

greschd
Copy link
Member

@greschd greschd commented Jun 15, 2020 •
edited
Loading

Updating the qe-tools dependency to the release candidate version 2.0.0rc1.

Significant changes:

  • The constants defined previously in qe_tools.constants are now a SimpleNamespace, importable as qe_tools.CONSTANTS.
  • The PwInputFile no longer accepts file handles or file names as input, just a single string containing the file contents. This is propagated through to the aiida-quantumespresso InputFile classes.
    @sphuber is it ok to change this also here, or should we add an __init__ to pwinputparse.PwInputFile that implements the current behavior?

@greschd greschd requested a review from sphuber June 15, 2020 08:39
greschd
greschd commented Jun 19, 2020
View reviewed changes
setup.json Show resolved Hide resolved
@greschd greschd marked this pull request as ready for review June 19, 2020 18:40
@greschd greschd mentioned this pull request Jun 24, 2020
Merged
4 tasks
@greschd greschd force-pushed the update_qe_tools branch 2 times, most recently from 65bd490 to 53dc4a1 Compare July 30, 2020 09:52
@greschd
Copy link
Member Author

greschd commented Jul 30, 2020

@sphuber when you have time, could you take a look at this? If you'd prefer a fully released version of qe-tools, I can do that also. Mainly, I'm waiting with that to see if there's any feedback / changes needed to qe-tools from this PR - it's a chicken-egg problem 😄.

@sphuber
Copy link
Contributor

sphuber commented Aug 20, 2020

Hey @greschd sorry for the delay. I think it would be better to wait for 2.0.0. If there really turns out to be a problem, it is easier to just release a patch version. We will just specify qe-tools~=2.0 such that it will automatically get installed. Experience with relying on release candidates is not the best.

@greschd greschd force-pushed the update_qe_tools branch 4 times, most recently from fa6f7e2 to 6a0cf8c Compare August 20, 2020 11:12
@greschd
Copy link
Member Author

greschd commented Aug 20, 2020

Would ~=2.0rc1 also work? If we just specify ~=2.0, the release candidate isn't considered as a valid match.

As far as releasing qe-tools==2.0.0, I don't think there is too much left to do. There's the constants issue aiidateam/qe-tools#25, and then it would be nice to add a bit of documentation (but that's not necessarily blocking).

What do think about the interface changes in qe-tools, from the perspective of aiida-quantumespresso?

@sphuber
Copy link
Contributor

sphuber commented Oct 7, 2020

@greschd what is the status of qe-tools? Is 2.0.0 already out? I was thinking of making the 3.2.0 release for aiida-quantumespresso, such that we can drop Python 3.5 for the next one. Just looking at what PRs we still want to include

@greschd
Copy link
Member Author

greschd commented Oct 7, 2020

Well, this is a bit of a chicken-egg problem: I'd want to get feedback on this PR before releasing 2.0.0 at least. In principle there are then still some issues labeled 2.0.0, but I'm not sure if any of them are really blocking.

I'm not sure if the "inconsistencies in physical constants" issue would still need fixing before release. The interface is now adapted so that we can handle multiple QE versions, so we could also fix the inconsistency itself later.

@greschd
Copy link
Member Author

greschd commented Oct 7, 2020

It would definitely be nice if we could get this PR and #503 (blocked only by this one) in though - if nothing else, so I can cross them off my mental to-do list 😄

@sphuber
Copy link
Contributor

sphuber commented Oct 7, 2020

Right, if you can update the branch and resolve the conflict I will merge this. Let's get this show on the road

Update to qe-tools 2.0.0rc1.
604c2b9 
The constants defined previously in 'qe_tools.constants' are
now a SimpleNamespace, importable as 'qe_tools.CONSTANTS'.
Furthermore, the 'PwInputFile' no longer accepts file handles or
file names as input, just a single string containing the file
contents. This is propagated through to the aiida-quantumespresso
InputFile classes.
@greschd
Copy link
Member Author

greschd commented Oct 8, 2020

Ready to rock 🤘

sphuber
sphuber approved these changes Oct 8, 2020
View reviewed changes
Copy link
Contributor

@sphuber sphuber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rock on

@greschd greschd merged commit 0978b66 into aiidateam:develop Oct 8, 2020
@greschd greschd deleted the update_qe_tools branch October 8, 2020 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sphuber sphuber sphuber approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@greschd @sphuber